feat(ai): BYOK provider layer; drop GRIDA_LOCALDEV_SUPERUSER#718
Conversation
Contributors had no way to exercise AI features locally without a Vercel AI Gateway key + Metronome billing wired up. The only escape was NEXT_PUBLIC_GRIDA_LOCALDEV_SUPERUSER, a public flag that bypassed both billing AND auth. Introduce a BYOK layer: when BYOK_OPENROUTER_API_KEY (OpenRouter via @ai-sdk/openai-compatible) or BYOK_AI_GATEWAY_API_KEY (dedicated AI Gateway key) is set, `grida`/`model` return a bare provider that structurally bypasses the billing seam (no gate, no Metronome ingest, no balance). The contributor's own key pays the provider directly. BYOK bypasses billing ONLY, never auth: requireOrganizationId and route/action auth always run. The old superuser flag is removed entirely, which makes the MissingOrgIdError/gate contract unconditional on the billed path (a net GRIDA-SEC-003 hardening). Behavior change: local AI without a BYOK key now requires real billing + always requires auth; the supported local path is "set a BYOK key". SECURITY.md documents the carve-out + residual risk; test/billing-quota-and-ai.md TC-035/036 retargeted.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR replaces the local-dev superuser bypass with a BYOK (Bring Your Own Keys) billing mode that enforces authentication and organization resolution while allowing contributors to bypass Metronome billing/metering. It updates the AI seam provider routing, credit state shapes, UI components, environment configuration, security boundaries, and comprehensive test coverage. ChangesBYOK Billing Bypass Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
softmarshmallow
added
ai
Under BYOK the seam forces balanceCents:0 and isByokActive() is server-only, so the credit chip/banner showed a misleading "$0 / out of credit / top up" while calls actually succeed via the contributor's own key. Plumb a `byok` flag from preloadAiCredits through the controller; useAiCredits() now returns a discriminated union where the `byok` variant omits cents/allowed/formatted, so every consumer is compile-forced to handle BYOK and render its own label. The low-balance / below-floor / top-up affordances are suppressed under BYOK across the AI chat, music, and image surfaces. Also correct a stale "non-superuser mode" comment in canvas generate.ts (GRIDA-SEC-003 hygiene from the superuser removal). GRIDA-SEC-003: isByokActive is re-exported through the seam (@/lib/ai/server); only the boolean reaches the client, never the key.
The "parses STAT table structure correctly" test inherited the 5000ms vitest default while synchronously parsing 4 large variable fonts, making it flake on slower CI runners (5084ms observed). Match its sibling multi-font tests with an explicit 20000ms timeout and drop a duplicate Recursive parse that added runtime but no coverage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/contributing/billing.md (1)
1-3: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd Markdown frontmatter with
format: md.This file is plain Markdown and should opt out of MDX parsing.
As per coding guidelines, "`docs/**/*.md`: For files that don't use JSX/MDX features, add `format: md` to frontmatter to opt out of MDX parsing entirely and prevent angle-bracket issues".Proposed fix
+--- +format: md +--- + # Contributing to Grida | Billing🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/contributing/billing.md` around lines 1 - 3, Add a YAML frontmatter block at the top of the "Contributing to Grida | Billing" Markdown file (the header line shown in the diff) that sets format: md so the file opts out of MDX parsing; insert the frontmatter before the existing "# Contributing to Grida | Billing" header and ensure it only contains format: md.
🧹 Nitpick comments (1)
test/billing-quota-and-ai.md (1)
202-210: 🏗️ Heavy liftMove BYOK billing-seam invariants to automated tests instead of manual UX cases.
Lines 202-210 describe server-side billing/auth logic, not human-interaction UX behavior; this should live in automated tests, with this manual file focused on interaction-only checks.
As per coding guidelines, "
test/**/*.md: Add manual test cases to thetestdirectory only for UX bugs requiring human interaction verification ... not for pure logic, math, or data transformations".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/contributing/billing.md`:
- Line 26: Replace the markdown link to ../../SECURITY.md in the sentence
starting with "Never set `BYOK_*` on a hosted or preview deploy." with inline
code (e.g., use ``../../SECURITY.md`` instead of a markdown link) so the docs
page no longer links outside /docs; update the sentence containing `BYOK_*`
accordingly and verify there are no other references linking outside /docs in
docs/contributing/billing.md.
---
Outside diff comments:
In `@docs/contributing/billing.md`:
- Around line 1-3: Add a YAML frontmatter block at the top of the "Contributing
to Grida | Billing" Markdown file (the header line shown in the diff) that sets
format: md so the file opts out of MDX parsing; insert the frontmatter before
the existing "# Contributing to Grida | Billing" header and ensure it only
contains format: md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2253d41f-9134-4bc2-9990-194f5bffd042
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
SECURITY.mddocs/contributing/billing.mdeditor/.env.exampleeditor/.oxlintrc.jsonceditor/app/(api)/private/ai/chat/route.tseditor/app/(canvas)/canvas/tools/ai/generate.tseditor/app/(tools)/(playground)/playground/image/_page.tsxeditor/app/(www)/(ai)/ai/_page.tsxeditor/app/(www)/(ai)/ai/music/playground/_page.tsxeditor/env.tseditor/lib/ai/__tests__/server.test.tseditor/lib/ai/credits/__tests__/controller.test.tseditor/lib/ai/credits/actions.tseditor/lib/ai/credits/controller.tseditor/lib/ai/credits/index.tseditor/lib/ai/credits/provider.tsxeditor/lib/ai/models.tseditor/lib/ai/server.tseditor/package.jsoneditor/scripts/audit-ai-seam.tspackages/grida-fonts/__tests__/typr.test.tstest/billing-quota-and-ai.md
💤 Files with no reviewable changes (1)
- editor/env.ts
| - Bypasses **billing only — never auth.** Still sign in (`insider@grida.co` / `password`); a resolvable org is still required (an unauthenticated request still 401s). | ||
| - **Text/chat only** — OpenRouter exposes no image/audio models. Catalog model IDs are unchanged; use IDs your provider accepts (edit `editor/lib/ai/models.ts` locally if one 404s). | ||
| - Precedence if both are set: OpenRouter, then AI Gateway. Fail-closed — an empty/unset key falls back to the billed path. | ||
| - **Never set `BYOK_*` on a hosted or preview deploy.** It disables billing **and** the org-id sanity gate for every org. Contributor / self-host / local only. See [SECURITY.md](../../SECURITY.md) `GRIDA-SEC-003` (BYOK carve-out). |
There was a problem hiding this comment.
Do not link to repository paths outside /docs from docs pages.
Line 26 links to ../../SECURITY.md; use inline code for that repo path instead of a markdown link.
Proposed fix
-- **Never set `BYOK_*` on a hosted or preview deploy.** It disables billing **and** the org-id sanity gate for every org. Contributor / self-host / local only. See [SECURITY.md](../../SECURITY.md) `GRIDA-SEC-003` (BYOK carve-out).
+- **Never set `BYOK_*` on a hosted or preview deploy.** It disables billing **and** the org-id sanity gate for every org. Contributor / self-host / local only. See `SECURITY.md` (`GRIDA-SEC-003`, BYOK carve-out).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Never set `BYOK_*` on a hosted or preview deploy.** It disables billing **and** the org-id sanity gate for every org. Contributor / self-host / local only. See [SECURITY.md](../../SECURITY.md) `GRIDA-SEC-003` (BYOK carve-out). | |
| - **Never set `BYOK_*` on a hosted or preview deploy.** It disables billing **and** the org-id sanity gate for every org. Contributor / self-host / local only. See `SECURITY.md` (`GRIDA-SEC-003`, BYOK carve-out). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/billing.md` at line 26, Replace the markdown link to
../../SECURITY.md in the sentence starting with "Never set `BYOK_*` on a hosted
or preview deploy." with inline code (e.g., use ``../../SECURITY.md`` instead of
a markdown link) so the docs page no longer links outside /docs; update the
sentence containing `BYOK_*` accordingly and verify there are no other
references linking outside /docs in docs/contributing/billing.md.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b4bc8efc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isByokActive()) { | ||
| // GRIDA-SEC-003 BYOK carve-out: no Grida balance to read. | ||
| return { | ||
| success: true, | ||
| data: { ...data, balanceCents: 0 } as AiActionData<T>, | ||
| }; |
There was a problem hiding this comment.
withAiAuth now returns balanceCents: 0 whenever any BYOK_* key is present, but several actions wrapped by withAiAuth still execute through billed paths (for example generateAudio calls methods.generateAudio, which uses runPrediction/withTransaction and still gates+ingests usage). In BYOK mode this makes clients treat credits as non-billed while those requests can still spend or block on real balance, so users can silently drain credit and then hit unexpected 402s. The BYOK shortcut should only skip balance reads for call paths that actually bypass the billing middleware.
Useful? React with 👍 / 👎.
| const openrouterKey = process.env.BYOK_OPENROUTER_API_KEY; | ||
| if (openrouterKey) { |
There was a problem hiding this comment.
The BYOK switch currently treats any truthy env string as active (if (openrouterKey) / if (aiGatewayKey)), so whitespace-only or malformed secrets still force the bare-provider path instead of falling back to billed mode. In that state all AI SDK calls can fail against an invalid upstream key while billing is bypassed, which is a hard outage caused by minor env formatting mistakes (e.g., a space/newline in secret value). Normalizing/validating keys (e.g., trim + non-empty) before enabling BYOK would preserve the intended fail-closed behavior.
Useful? React with 👍 / 👎.
Per-test timeout bumps were whack-a-mole: after stabilizing the STAT test, "parses Geist variable font" then flaked at 5092ms. Any single large variable-font parse can exceed the 5000ms vitest default on a loaded CI runner, so raise it package-wide to 30000ms and drop the now-redundant per-test override.
Addresses the PR #718 AI review (Codex P1/P2, CodeRabbit). P1 (silent drain): `withAiAuth`'s `balanceCents:0` short-circuit fired for every action whenever a BYOK key was set, but BYOK only swaps the AI-SDK provider — Replicate-backed actions (audio/image) still run `withTransaction` and bill. Those calls spent real credit and could 402 while the client was told "0 / unlimited". The short-circuit is now opt-gated (`byokBypass`, default `false`); only `ai/chat` (AI-SDK text) sets it. Billed actions read the real balance under BYOK. UI: dropped the global discriminated union — it wrongly forced a "BYOK" label onto Replicate-billed surfaces. `useAiCredits()` returns a flat `byok` flag; only the AI chat page acts on it. Music & image playgrounds reverted to the normal balance view. P2: BYOK keys are trimmed; whitespace-only falls back to the billed path (strengthens the documented fail-closed contract). Docs: SECURITY.md GRIDA-SEC-003 + billing.md reworded — BYOK bypass is the AI-SDK text path only; Replicate audio/image still gate+bill. billing.md outside-/docs links → GitHub URLs; added `format: md`. GRIDA-SEC-003: net hardening — narrows the bypass, never widens it; auth + the unconditional billed-path gate are untouched.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@editor/lib/ai/actions/chat.ts`:
- Around line 103-108: The code currently accepts any catalog id (isCatalogId)
so callers can request hidden/reserved models; change the server-side check that
sets useModelId so it only allows models present in the public/tiered allowlist
(i.e., the same set used to render the picker) rather than any catalog entry.
Concretely, replace the isCatalogId(requested) predicate with a check against
the tiered/public model set (for example verify requested matches one of
catalog[tiers.*].id entries or add a helper like isTieredModel(requested)), then
keep the existing logic that picks grida(useModelId) vs model("mini") and
resolves resolvedId from the tier catalog to avoid desync. Ensure this
validation runs in the same function where requested/useModelId are computed so
hidden models cannot be selected by forging the payload.
In `@editor/lib/ai/server.ts`:
- Around line 443-453: The current switch assigns activeProvider = byok ??
wrappedProvider which causes gridaFn.imageModel to call the BYOK provider and
bypass billing; instead keep languageModel bound to the active provider but
force imageModel to always use the billing-wrapped gateway. Update gridaFn and
its properties so gridaFn(modelId) and gridaFn.languageModel(modelId) use
activeProvider.languageModel(modelId) while gridaFn.imageModel(modelId) always
delegates to wrappedProvider.imageModel(modelId) (so methods.getSDKImageModel()
remains metered).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcc5cc63-ec1e-4f75-888b-1f4b58d8106f
📒 Files selected for processing (8)
SECURITY.mddocs/contributing/billing.mdeditor/app/(www)/(ai)/ai/_page.tsxeditor/lib/ai/__tests__/server.test.tseditor/lib/ai/actions/chat.tseditor/lib/ai/credits/provider.tsxeditor/lib/ai/models.tseditor/lib/ai/server.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- editor/lib/ai/models.ts
- editor/lib/ai/tests/server.test.ts
CodeRabbit follow-up on 772b15a: - grida.imageModel() stays on the billing-wrapped provider; BYOK swaps only the language provider, so SDK image generation is still gated + metered under BYOK (matches SECURITY.md / billing.md GRIDA-SEC-003). - runChat only accepts the 4 tier model ids (was: any catalog id), so a forged payload can't request reserved/non-tiered models.
1 participant
Summary
BYOK_OPENROUTER_API_KEY(OpenRouter via@ai-sdk/openai-compatible) orBYOK_AI_GATEWAY_API_KEY(a dedicated Vercel AI Gateway key) is set,grida/modelreturn a bare provider that structurally bypasses the billing seam — no gate, no Metronome ingest, no balance read. The contributor's own key pays the provider. Fail-closed: empty/unset ⇒ the billed path. Precedence: OpenRouter, then AI Gateway.NEXT_PUBLIC_GRIDA_LOCALDEV_SUPERUSERremoved entirely (it was AI-only). Its billing-bypass and auth-bypass holes are gone;checkGate/MissingOrgIdErrorare now unconditional on the billed path — a net GRIDA-SEC-003 hardening.requireOrganizationId+ route/action auth always run (verified live: unauth → 401).Security — why this is safe
This cannot be enabled on, or leaked from, the hosted product:
NEXT_PUBLIC_.BYOK_OPENROUTER_API_KEY/BYOK_AI_GATEWAY_API_KEYare read viaprocess.envin server-only seam code (editor/lib/ai/models.ts). Next.js only inlinesNEXT_PUBLIC_*into the client bundle — these are never shipped to the browser. Zero secret-leakage surface.OPENAI_API_KEY/REPLICATE_API_TOKENsecrets.NEXT_PUBLIC_GRIDA_LOCALDEV_SUPERUSERwas a public flag that bypassed billing and auth. Replacing it with a server-only secret that bypasses billing only (auth always enforced) removes a public bypass and tightens the billed path (MissingOrgIdError/gate now unconditional).byokresolves tonullunless a key env var is a non-empty string → any ambiguity falls back to the billed path. Resolved once at module load (no per-request attack surface).SECURITY.mdGRIDA-SEC-003 records the intentional billing-only bypass and the residual risk (aBYOK_*secret must never be set on a hosted/preview deploy — operational, by the same trust model as every other server secret).Behavior change (breaking for local dev)
Local AI without a BYOK key now requires real billing (gate + Metronome) and always requires auth (login + resolvable org). The superuser no-auth/no-billing loop is gone by design — the supported local path is "set a BYOK key (still authenticated)".
docs/contributing/billing.mddocuments the BYOK shortcut;SECURITY.mdGRIDA-SEC-003 documents the carve-out + residual risk;test/billing-quota-and-ai.mdTC-035/036 retargeted.Test plan
turbo typecheck --filter=editor --force— 29/29withAiAuthtest: auth runs,refreshBalanceskipped,balanceCents:0)oxlint0 errors ·audit-ai-seam.ts0 violations (1862 files) · repo grep: no superuser refs.env.local, seam module graph loads (no 500 ⇒createOpenAICompatibleconstruction sound);POST /private/ai/chatunauth → 401 (auth enforced)assertOrgMemberpath)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security
Documentation